improvement: set acceptAnonymousUploads=1 for initial kotsadm-tls cert #4344
Conversation
| @@ -53,8 +52,6 @@ type cert struct { | |||
| func main() { | |||
| log.Printf("Commit %s\n", os.Getenv("COMMIT")) | |||
|
|
|||
| rand.Seed(time.Now().UnixNano()) | |||
There was a problem hiding this comment.
Why remove this?
Accidental clean up as I was removing deprecated code. I meant to put it back.
We however do not need it. It was deprecated in go1.20. The runtime automatically generates a seed for us. Also, I do not see anywhere where we use random numbers.
There was a problem hiding this comment.
I think we are safe to remove this unless there are objections.
I reviewed the code and do not see any reason for keeping it
kurl_proxy/cmd/main.go
Outdated
| // TODO: Why is the namespace always "default"? | ||
| // Requests to kotsadm.embeddded-cluster.svc.cluster for example will | ||
| // fail with invalid cert errors |
There was a problem hiding this comment.
Not sure about the historical reasons, but kots and kurl-proxy are always deployed to the default namespace with the kurl add-on. If it needs to be other namespaces, i think we could make it configurable. Maybe even use the namespace variable we already have in the main function that comes from here?
There was a problem hiding this comment.
I'll add a change to allow overriding the namespace
There was a problem hiding this comment.
I left the hostname as-is, but I have added DNS alt names to the cert in case the namespace is not default. This will keep any old assumptions valid such as in ekco's rotation logic
|
|
||
| err = generateDefaultCertSecret(secrets, namespace) | ||
| if err != nil { | ||
| log.Printf("Could not regenerate default certificate: %v", err) | ||
| } | ||
| // TODO: Why are we exiting here? It leads to a pod restart |
There was a problem hiding this comment.
not sure. on the surface, it seems like we dont need a restart for anything
There was a problem hiding this comment.
I'll leave that behaviour for now, but leave the TODO there so as not to introduce some unwanted behaviour change.
banjoh
force-pushed
the
em/sc-94724/set-acceptAnonymousUploads-to-1
branch
from
55eb9bd to
1eba4b9
Compare
What this PR does / why we need it:
Have
kurl-proxyservice setacceptAnonymousUploads=1in thekotsadm-tlscert by default when we generate the default self signed certificate. This allows prompting Admin Console users to upload BYO certs on first for first time installs. When KOTS is installed as a kURL addon, the installer script sets this flag to 1. Doing so in this service aligns with what's expected.Which issue(s) this PR fixes:
Fixes # sc-94724
Special notes for your reviewer:
Steps to reproduce
Does this PR introduce a user-facing change?
Does this PR require documentation?
Review notes